Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP adding security resource sharing SPI and sample-extension-plugin #26

Open
wants to merge 86 commits into
base: main
Choose a base branch
from

Conversation

cwperks
Copy link
Owner

@cwperks cwperks commented Aug 7, 2024

This PR introduces a SPI from the Security Plugin that others plugins can extend to get resource access control provided by the security plugin.

It relies on this core PR to make extended plugins optional:

It also relies on this core PR to allow the security plugin to write integration tests with classpath extensible plugins:

Background:

In the default distribution of OpenSearch, there are many instances of plugins implementing custom resource access control because the security plugin does not provide a mechanism to secure resources created by plugins. For example, a simple search for filter_by_backend_roles on the documentation website shows a few plugins that implement custom resource access control. filter_by_backend_role is a very simplistic access control mechanism that plugins use that restrict what resources are listed when an authenticated user is on a page is OpenSearch Dashboards that lists that specific type of resource.

For instance, in ISM when a user navigates to the page that lists policies, the page will either list:

  1. Policies created by the logged in user
  2. Policies shared to the logged in user by backend role

Any other policies would not be displayed and cannot be interacted with by the user.

In this simple model of resource access control, what a user can do with a resource is determined by the roles that user is mapped to and not determined by the user sharing their resource with another user. For instance, if a user is mapped to the anomaly_detection_full_access role, then that user will have full access to any detector shared with the user. The user sharing the detector has no mechanism to specify that the user that they are sharing the detector with only has read access to the detector that they have ownership over.

One other shortfall of the current resource access control model is that plugins end up copying the user from the ThreadContext at the time that a resource is created and store the copy in the resource's metadata. Any changes to the user are not propagated.

SPI:

This PR lays the foundation for providing a consistent resource access control experience across plugins in the default distribution.

This particular PR provides an off ramp for the current simple resource access control prevalent across plugins and centralizes the access control to the security plugin.

Plugins can transition to using this SPI and maintain backward compatibility with the current resource access control model filter_by_backend_role.

In 3.0.0, I would like to change the resource access control model to one where the owner of a resource can specify the level of access of a resource when sharing with other users on the platform.

For example, imagine a Searchable Photo Album Plugin. In this plugin, a user can create photo albums, upload photos to an album, add captions, tag images, and leave comments.

  1. The creator of an album has full access to their own album
  2. When sharing the album with others, the owner of the album should be the one to determine the access level with others. For instance, the user should be able to specify Read Only, Read + Comment or Full Access when sharing with other users on the platform

Number 2 is not possible in the current access control model. In the current access control model, if a user has searchable_photo_album_full_access role, then they will have full access over any album shared to them and the user sharing their album has no control over the level of access to the album that other users have.

For Plugin Developers:

For plugin developers, add a dependency on the security plugin SPI and then create an implementation of ResourceSharingExtension which needs 4 (maybe 3 minus the ResourceParser) methods implemented:

@Override
public String getResourceType() {
    return "sample_resource";
}

@Override
public String getResourceIndex() {
    return RESOURCE_INDEX_NAME;
}

@Override
public ResourceParser<? extends Resource> getResourceParser() {
    return new SampleResourceParser();
}

@SuppressWarnings("unchecked")
@Override
public void assignResourceSharingService(ResourceSharingService<? extends Resource> service) {
    SampleResourceSharingServiceProvider.getInstance().set((ResourceSharingService<SampleResource>) service);
}

Within createComponents, check to see if security is installed (i.e. assignResourceSharingService is called) or else create an instance of the DefaultResourceSharingService. With the DefaultResourceSharingService, all users have access to any resource regardless of who creates the resource.

@Override
public Collection<Object> createComponents(
    ...
) {
    return List.of(SampleResourceSharingServiceProvider.getInstance());
}

This ResourceSharingService can then be used by plugin developers to determine whether the current user has access to a resource. The interface is defined like this:

public interface ResourceSharingService<T extends Resource> {
    void isSharedWithCurrentUser(String resourceId, ActionListener<Boolean> shareListener);
}

Testing:

Run tests using ./gradlew :opensearch-security-sample-extension:integTest -x test -x integrationTest -x spotbugsIntegrationTest -Dtests.opensearch.username=admin -Dtests.opensearch.password=admin -Duser=admin -Dpassword=admin -Dhttps=true -Dsecurity=true -Dtests.opensearch.secure=true -i

cwperks added 14 commits August 13, 2024 17:09
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
…ble GetResourceTransportAction

Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant